Fix TypeScript compilation errors in notification and workflow hooks#27
Fix TypeScript compilation errors in notification and workflow hooks#27
Conversation
- Import GetNotificationPreferencesResponse and UpdateNotificationPreferencesResponse from @objectstack/spec/api instead of @objectstack/client - Fix test type assertion for NotificationPreferences - Add missing 'reason' parameter to workflow reject() method signature and implementation - Update test expectations to match new reject() signature Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses CI TypeScript failures by aligning hook typings with the ObjectStack API spec—fixing missing type exports for notifications preferences and updating the workflow reject signature to require a reason.
Changes:
- Import notification preference response types from
@objectstack/spec/apiinstead of@objectstack/client. - Update
useWorkflowState().rejectto require areasonparameter and pass it through toclient.workflow.reject. - Adjust related tests to match updated typings / request shapes.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| hooks/useWorkflowState.ts | Updates the hook’s reject signature and request payload to include required reason. |
| hooks/useNotifications.ts | Switches preference response type imports to @objectstack/spec/api. |
| tests/hooks/useWorkflowState.test.ts | Updates reject test to pass/expect reason. |
| tests/hooks/useNotifications.test.ts | Updates preference test typing to match the hook return type. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| expect(mockReject).toHaveBeenCalledWith({ | ||
| object: "tasks", | ||
| recordId: "rec-1", | ||
| comment: "Needs changes", | ||
| reason: "Needs changes", | ||
| }); |
There was a problem hiding this comment.
This expectation will fail because the hook calls client.workflow.reject({ object, recordId, reason, comment }), so the argument object includes a comment key (as undefined when omitted). Update the assertion to either include comment: undefined or use a partial matcher (e.g., expect.objectContaining) so it matches the actual call shape.
| import type { | ||
| ListNotificationsResponse, | ||
| RegisterDeviceResponse, | ||
| } from "@objectstack/client"; | ||
| import type { | ||
| GetNotificationPreferencesResponse, | ||
| UpdateNotificationPreferencesResponse, | ||
| } from "@objectstack/client"; | ||
| } from "@objectstack/spec/api"; |
There was a problem hiding this comment.
@objectstack/spec/api is now a direct import. @objectstack/spec is not listed in package.json dependencies (only present via pnpm-lock as a transitive dep), so this can become brittle if the client package ever stops depending on it. Add @objectstack/spec as an explicit dependency (or switch to a type source that’s already a direct dependency) to keep installs/builds stable.
CI failing with 4 TypeScript errors: missing type exports and incorrect workflow reject signature.
Type Import Corrections
GetNotificationPreferencesResponseandUpdateNotificationPreferencesResponseare not re-exported by@objectstack/client. Changed to import from@objectstack/spec/api:Workflow Reject Signature
API spec requires
reasonparameter (not optional). Updated interface and implementation:Implementation now passes all required fields to
client.workflow.reject():Test Updates
reasonfieldOriginal prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.